Skip to content

Fix Bitcoin Core build errors from libmultiprocess headers - #370

Merged
ryanofsky merged 4 commits into
bitcoin-core:masterfrom
ryanofsky:pr/sockinline
Oct 13, 2026
Merged

ryanofsky merged 4 commits into
bitcoin-core:masterfrom
ryanofsky:pr/sockinline

Conversation

@ryanofsky

Copy link
Copy Markdown
Collaborator

Problem: Bumping the libmultiprocess subtree in Bitcoin Core to current master breaks the build with -Werror. GCC reports SocketError as an unused const variable in every file that includes mp/util.h without using it (-Wunused-const-variable, enabled by bitcoin/bitcoin#36275; CI failure), and the #if KJ_NO_EXCEPTIONS check added in #364 fails -Wundef when the macro is not defined.

Solution: Make SocketError an inline constexpr variable, the same change Bitcoin Core made to its own SocketError in bitcoin/bitcoin#35852, and check defined(KJ_NO_EXCEPTIONS) before testing its value. Also enable both warnings in CI here (-Wunused-const-variable in the GCC configs, -Wundef in default and llvm) so these problems are caught before a subtree update. Details are in the commit messages.

Declare SocketError as an inline variable so the header does not define a
separate internal-linkage copy in every translation unit that includes it.
Bitcoin Core's build now enables -Wunused-const-variable (bitcoin/bitcoin#36275),
which with GCC warns about unused const variables defined in headers and
fails the build with -Werror in every file that includes mp/util.h without
using SocketError. Bitcoin Core made the same change to its own SocketError
definition in bitcoin/bitcoin#35852.

This change was written with Claude Opus 5.5 (1M context).
Bitcoin Core builds the libmultiprocess subtree with -Wunused-const-variable
(bitcoin/bitcoin#36275). With GCC this warns about unused const variables
defined in headers, so a header-level constexpr that is not inline breaks the
Bitcoin Core build. Enable it in the GCC-based CI configs to catch this here
first. Clang's -Wunused-const-variable does not warn about variables defined
in headers, so it is not added to the clang configs.

This change was written with Claude Opus 5.5 (1M context).
Check that KJ_NO_EXCEPTIONS is defined before testing its value. Bitcoin
Core builds the libmultiprocess subtree with -Wundef, so the plain
`#if KJ_NO_EXCEPTIONS` check added in bitcoin-core#364 fails the build with -Werror
whenever the macro is not defined, which is the normal case.

This change was written with Claude Opus 5.5 (1M context).
Bitcoin Core builds the libmultiprocess subtree with -Wundef, so a
preprocessor check on an undefined macro breaks the Bitcoin Core build.
Enable it in one GCC and one clang config to catch this here first.

This change was written with Claude Opus 5.5 (1M context).
@DrahtBot

DrahtBot commented Oct 2, 2026 •

Copy link
Copy Markdown

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers
ACK maflcko, ViniciusCestarii, hebasto, Sjors, xyzconstant

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #342 (Allow request cancellation for wrapped C++ methods by xyzconstant)
  • #231 (Add windows support by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot mentioned this pull request Oct 2, 2026
@maflcko

maflcko commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

lgtm ACK 3577d23

@ViniciusCestarii ViniciusCestarii left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 3577d23

Confirmed Bitcoin Core builds with libmultiprocess with these changes

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 3577d23, I have reviewed the code and it looks OK.

@Sjors

Sjors commented Oct 7, 2026

Copy link
Copy Markdown
Member

ACK 3577d23

Sjors added a commit to stratum-mining/sv2-tp that referenced this pull request Oct 7, 2026
Backport of bitcoin-core/libmultiprocess#370.

Import only the two header fixes so the subtree builds with
-Wundef and -Wunused-const-variable enabled as errors. The upstream
CI changes are not included.
@xyzconstant

Copy link
Copy Markdown
Contributor

ACK 3577d23

Reviewed commit by commit, LGTM.

@ryanofsky
ryanofsky merged commit 2e7812a into bitcoin-core:master Oct 13, 2026
14 checks passed
Sjors added a commit to Sjors/sv2-tp that referenced this pull request Oct 13, 2026
Sync with upstream master after bitcoin-core/libmultiprocess#370
landed. The header fixes were already imported by sv2-tp#134; this
update adds the upstream CI warning flags and IWYU fixes from #371.

Use the shared subtree exclusions in the shell locale linter so
upstream scripts are not required to follow local locale conventions.

Base the subtree merge on the previous import so the squash and
merge commits remain reusable for backports.
Sjors added a commit to Sjors/sv2-tp that referenced this pull request Oct 13, 2026
Sync with upstream master after bitcoin-core/libmultiprocess#370
landed. The header fixes were already imported by sv2-tp#134; this
update adds the upstream CI warning flags and IWYU fixes from #371.

Base the subtree merge on the previous import so the squash and
merge commits remain reusable for backports.
Sjors added a commit to stratum-mining/sv2-tp that referenced this pull request Oct 13, 2026
2e7812a5a1 Merge bitcoin-core/libmultiprocess#370: Fix Bitcoin Core build errors from libmultiprocess headers
686bccf766 Merge bitcoin-core/libmultiprocess#371: cmake: Fix silent include-what-you-use failures
f0a14754ef cmake: Show include-what-you-use diagnostics when it fails
92127a984a cmake: Pass -w to include-what-you-use
3577d23f61 ci: Enable -Wundef in default and llvm jobs
debd42955e proxy-types: Avoid -Wundef error when KJ_NO_EXCEPTIONS is not defined
a8c64c7faa ci: Enable -Wunused-const-variable in GCC jobs
701449c165 util: Make SocketError inline constexpr
REVERT: fcc569c2cf proxy-types: Avoid -Wundef error when KJ_NO_EXCEPTIONS is not defined
REVERT: fdeefd7aab util: Make SocketError inline constexpr

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 2e7812a5a1ea8fada26f74b06be62e195a3783a5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants